Conversation
for more information, see https://pre-commit.ci
This comment was marked as outdated.
This comment was marked as outdated.
Oh man, yes we can do that! In a way that's the largest diff 🤣 RE: typing, do you have any time/willing to take a stab at it? |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Thanks @FBruzzesi, really appreciate the slog you've gone through on this one 😍
As mentioned on discord, most of the suggestions are about this:
Current:
- zip(...)
+ zip(..., strict=True)Proposed:
- zip(...)
+ zip(..., strict=False)We can always make more of them strict later if needed 🙂
Note
zip_strict -> zip(..., strict=True) is all good
|
@MarcoGorelli let me know if you are happy with this. I am happy to make a release before merging, so that some of the most recent features make it into a version that still support py3.9 |
|
yeah no objections from my end if you're both happy with this 🚀 |
EdAbati
left a comment
EdAbati
left a comment
There was a problem hiding this comment.
thank you for this :) 🎉 and goodbye Python 3.9!
added 2 very minor (non blocking) comments
| other=other, | ||
| columns_to_select=list(right_on), | ||
| columns_mapping=dict(zip(right_on, left_on)), | ||
| columns_mapping=dict(zip(right_on, left_on, strict=True)), |
There was a problem hiding this comment.
Not sure if this is deliberate, but it was not strict before
I think it would actually make sense to make it strict, but did I understand correctly you wanted to do it in a follow-up? There we could align all columns_mapping to bestrict=True
4 participants
Description
It's likely that I have missed some branches that could be simplified due to newer python and bumping upstream min versions. Take this as a start to discuss in more detail
What type of PR is this? (check all applicable)
Related issues
3.9#3204